Skip to content

Implement HW/SW crypto affinity#281

Open
AlexLanzano wants to merge 10 commits intowolfSSL:mainfrom
AlexLanzano:crypto-hw-sw-affinity
Open

Implement HW/SW crypto affinity#281
AlexLanzano wants to merge 10 commits intowolfSSL:mainfrom
AlexLanzano:crypto-hw-sw-affinity

Conversation

@AlexLanzano
Copy link
Member

@AlexLanzano AlexLanzano commented Feb 4, 2026

Add SetCryptoAffinity API for runtime HW/SW crypto selection

Summary

This PR adds a new client API to dynamically switch between hardware and software cryptographic implementations at runtime. This allows clients to control whether the server uses crypto callbacks (hardware acceleration) or pure software wolfCrypt implementations.

Changes

  • New Client API (wh_client.c, wh_client.h)

    • wh_Client_SetCryptoAffinity() - blocking call
    • wh_Client_SetCryptoAffinityRequest() / wh_Client_SetCryptoAffinityResponse() - async API
  • New Message Types (wh_message_comm.c, wh_message_comm.h)

    • WH_MESSAGE_COMM_ACTION_SET_CRYPTO_AFFINITY action
    • WH_CRYPTO_AFFINITY_SW / WH_CRYPTO_AFFINITY_HW enum values
    • Request/response structures with translation functions
  • Server Handler (wh_server.c, wh_server.h)

    • Added configDevId field to preserve the original configured device ID
    • Handler switches devId between INVALID_DEVID (SW) and configDevId (HW)
  • Tests (wh_test_clientserver.c)

    • Added _testCryptoAffinity() to verify SW/HW switching behavior
  • Documentation (docs/draft/crypto_affinity.md)

    • API usage guide with examples

Usage

int32_t server_rc;
uint32_t affinity;

// Switch to software crypto
wh_Client_SetCryptoAffinity(client, WH_CRYPTO_AFFINITY_SW, &server_rc, &affinity);

// Switch to hardware crypto (if configured)
wh_Client_SetCryptoAffinity(client, WH_CRYPTO_AFFINITY_HW, &server_rc, &affinity);

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements a new client API to dynamically switch between hardware and software cryptographic implementations at runtime. The feature allows clients to control whether the server uses hardware acceleration (via crypto callbacks) or pure software wolfCrypt implementations by modifying the server's devId field.

Changes:

  • New client API with both blocking and async variants for setting crypto affinity
  • Server handler to process affinity change requests with proper validation and error handling
  • New message types and translation functions for client-server communication
  • Comprehensive test coverage including edge cases and error conditions

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
wolfhsm/wh_server.h Added configDevId field to preserve original device ID configuration
wolfhsm/wh_message_comm.h Defined message action, enum values, and request/response structures
wolfhsm/wh_error.h Added WH_ERROR_BADCONFIG error code for configuration failures
wolfhsm/wh_client.h Declared new client API functions with comprehensive documentation
src/wh_server.c Implemented initialization of configDevId and request handler with validation
src/wh_message_comm.c Implemented message translation functions following existing patterns
src/wh_client.c Implemented client APIs with proper error handling and retry logic
test/wh_test_clientserver.c Added comprehensive test coverage for various scenarios
docs/draft/crypto_affinity.md Created API documentation with usage examples and return code reference

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@AlexLanzano AlexLanzano force-pushed the crypto-hw-sw-affinity branch from 9949065 to 506f420 Compare February 5, 2026 19:08
@AlexLanzano AlexLanzano requested a review from Copilot February 5, 2026 19:18
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Contributor

@bigbrett bigbrett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work. Some tweaks required and some deeper architectural questions.

@bigbrett bigbrett marked this pull request as draft February 10, 2026 15:49
@AlexLanzano AlexLanzano force-pushed the crypto-hw-sw-affinity branch 3 times, most recently from d426556 to b109054 Compare February 10, 2026 19:31
@AlexLanzano AlexLanzano requested a review from Copilot February 10, 2026 19:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 29 out of 30 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (1)

wolfhsm/wh_server.h:72

  • Removing devId from whServerCryptoContext is an API/ABI breaking change for downstream users that initialize this public struct (previously .devId = INVALID_DEVID). If this is intentional, it should be called out in release notes/migration docs; otherwise consider providing a compatibility shim (e.g., keep the field behind a deprecated macro) to avoid breaking existing integrations.
#ifndef WOLFHSM_CFG_NO_CRYPTO

typedef struct whServerCryptoContext {
#ifndef WC_NO_RNG
    WC_RNG rng[1];
#endif
} whServerCryptoContext;

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@AlexLanzano AlexLanzano marked this pull request as ready for review February 10, 2026 19:53
@AlexLanzano AlexLanzano requested a review from bigbrett February 10, 2026 19:53
@bigbrett bigbrett force-pushed the crypto-hw-sw-affinity branch from c72c40c to cbc3ea7 Compare February 18, 2026 00:14
@AlexLanzano AlexLanzano force-pushed the crypto-hw-sw-affinity branch 2 times, most recently from 14fa4e5 to 4de6b72 Compare February 21, 2026 04:09
@AlexLanzano AlexLanzano requested a review from bigbrett February 21, 2026 04:14
Copy link
Contributor

@bigbrett bigbrett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexLanzano looks awesome. Pls remove unnecessary include and run clang-format, then we are good.


/* Component includes */
#include "wolfhsm/wh_comm.h"
#include "wolfhsm/wh_message_comm.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary include

whCommServer comm[1];
#ifndef WOLFHSM_CFG_NO_CRYPTO
whServerCryptoContext* crypto;
int defaultDevId;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run git-clang-format main and commit changes pls

Copilot AI review requested due to automatic review settings March 6, 2026 18:52
@AlexLanzano AlexLanzano force-pushed the crypto-hw-sw-affinity branch from 4de6b72 to 78735dd Compare March 6, 2026 18:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 32 out of 32 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

wolfhsm/wh_server.h:1

  • The field is named defaultDevId while the existing codebase consistently uses devId (e.g., whServerConfig.devId, crypto->devId). A more consistent name following the existing convention would be configDevId as mentioned in the PR description, or simply match the pattern used elsewhere. defaultDevId implies a fallback semantic that is not always accurate (e.g., it is also used directly for SHE and keystore operations regardless of affinity).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings March 7, 2026 14:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 32 out of 32 changed files in this pull request and generated 7 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@AlexLanzano AlexLanzano requested a review from bigbrett March 7, 2026 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants